Skip to content

feat(spend): cache-first and 5m TTL for dashboard - #3107

Open
Yuxin-Qiao wants to merge 21 commits into
steipete:mainfrom
Yuxin-Qiao:feat/spend-cache-ttl
Open

feat(spend): cache-first and 5m TTL for dashboard#3107
Yuxin-Qiao wants to merge 21 commits into
steipete:mainfrom
Yuxin-Qiao:feat/spend-cache-ttl

Conversation

@Yuxin-Qiao

@Yuxin-Qiao Yuxin-Qiao commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

test body

@clawsweeper

clawsweeper Bot commented Aug 20, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1f4bbb2cc7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +73 to +77
if !force,
let lastAt = self.lastSpendDashboardTokenFetchAt[provider.instanceID],
let lastScope = self.lastSpendDashboardTokenFetchScope[provider.instanceID],
lastScope == costScopeSignature,
self.spendDashboardTokenSnapshotPublicationForCurrentConfig(for: provider) != nil,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Route a non-forced request through the TTL check

This TTL cannot fire through production code: the sole caller in SpendDashboardSource.makeRequest always passes force: true, while .refreshMissing invokes that caller only when no current publication exists—even though this condition requires one. Consequently, the new five-minute guard cannot suppress any dashboard token scan; the caller needs to preserve the build mode's forced/non-forced semantics or perform the TTL decision before the missing-publication predicate.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 20, 2026
@clawsweeper

clawsweeper Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 23, 2026, 9:14 AM ET / 13:14 UTC.

ClawSweeper review

What this changes

The PR adds cache-first Spend dashboard loading, a five-minute freshness policy, aggregate SQLite hydration, and related Codex/OpenCodex accounting updates.

Merge readiness

Blocked until real behavior proof is added - 7 items remain

Keep this PR open: three previously reported current-head defects remain, and it still lacks real after-fix dashboard proof.

Priority: P2
Reviewed head: 5fdf6d7d05db6ac7b2c3ea769ec4bbd49cd0dd9e

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The patch contains useful targeted tests but remains blocked by three correctness defects and missing real behavior proof.
Proof confidence 🧂 unranked krab (1/6) Needs real behavior proof before merge: The PR body is only “test body,” and the prepared images belong to another branch rather than this head, so there is no redacted after-fix dashboard run proving cache use, TTL expiry, or upgraded-cache behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦐 gold shrimp (3/6) 3 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR body is only “test body,” and the prepared images belong to another branch rather than this head, so there is no redacted after-fix dashboard run proving cache use, TTL expiry, or upgraded-cache behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 6 items Current main does not contain the TTL implementation: Current main has the earlier refresh method but no stale-dashboard predicate; the PR head adds the TTL path and aggregate loading, so the PR is not obsolete.
TTL timestamp is never recorded after a successful fetch: The PR’s success path records only the completed scope before publishing; without a last-fetch time, an existing valid publication is treated as fresh indefinitely.
Exclusive-output representation conflicts with analytics invariant: The PR stores output after subtracting reasoning, while the downstream analytics fragment still clamps reasoning to output, losing reasoning whenever it exceeds the exclusive remainder.
Findings 3 actionable findings [P1] Restore exact cutover parity assertion
[P2] Record successful fetch completion time
[P2] Preserve reasoning in project analytics
Security None None.

Live Verification

Command: swift run CodexBarCLI dashboard --help

Result: FAIL (failed) — execution before step 1 run: sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

Assertions:

  • FAIL expect_output: dashboard

How this fits together

The Spend dashboard combines provider snapshots and locally cached usage into the menu-bar app’s spend views. This PR changes how those inputs are refreshed and how stored Codex/OpenCodex usage is reconstructed into dashboard reports.

flowchart LR
  A[Provider usage and local logs] --> B[Usage snapshots]
  C[SQLite usage cache] --> D[Aggregate report hydration]
  B --> E[Freshness decision]
  D --> F[Spend dashboard model]
  E --> F
  F --> G[Dashboard pane]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR body is only “test body,” and the prepared images belong to another branch rather than this head, so there is no redacted after-fix dashboard run proving cache use, TTL expiry, or upgraded-cache behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Restore exact cutover parity assertion (P1) - outputTokens is now already exclusive of reasoning, so subtracting reasoningTokens again undercounts the fixture. Replacing equality with a 50,000-token tolerance then masks that regression; count the exclusive output once and restore exact parity.
  • Record successful fetch completion time (P2) - The new stale predicate needs lastSpendDashboardTokenFetchAt, but this success path only writes the scope and then publishes. Once a current publication exists, the nil-timestamp branch returns false indefinitely, so reopening after five minutes never refreshes; set the time after a successful or confirmed-empty publication only.
  • Preserve reasoning in project analytics (P2) - This change passes exclusive output downstream, but CodexModelsUsageFragment still clamps reasoning to output. An event with 100 total output and 60 reasoning becomes (output: 40, reasoning: 40) in analytics; update that invariant and add a regression for reasoning larger than the exclusive remainder.
  • Resolve merge risk (P1) - Merging would make the advertised five-minute expiry ineffective after a successful dashboard publication, allowing stale provider spend data to persist.
  • Resolve merge risk (P1) - The new stored exclusive-output semantics can underreport reasoning in project/model analytics.
  • Resolve merge risk (P1) - The SQLite schema/cache behavior needs an upgraded-cache real run before merge.

Findings

  • [P1] Restore exact cutover parity assertion — Tests/CodexBarTests/CostUsageStoreCutoverTests.swift:78-85
  • [P2] Record successful fetch completion time — Sources/CodexBar/UsageStore+SpendDashboardTokenCost.swift:146-152
  • [P2] Preserve reasoning in project analytics — Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift:4384-4389
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch surface 32 files affected The change crosses dashboard refresh logic, local cache persistence, report reconstruction, and focused tests.
Production versus tests production +369/-86, tests +651/-74 The implementation is large enough that cache and accounting regressions need focused upgrade and runtime evidence.

Merge-risk options

Maintainer options:

  1. Repair and prove cache semantics (recommended)
    Restore exact accounting assertions and completion-based freshness recording, then show a redacted dashboard run across cold, warm, expired, and upgraded cache states.

Technical review

Best possible solution:

Restore exact accounting and reasoning invariants, record freshness only after a completed publication, then show a redacted fresh-bundle dashboard run for cold cache, in-TTL reuse, expiry refresh, and upgraded-cache behavior.

Do we have a high-confidence way to reproduce the issue?

Yes for the code paths: source inspection shows the missing post-success timestamp and the exclusive-output analytics truncation, while focused tests can establish regressions. A real dashboard run is still missing.

Is this the best way to solve the issue?

No; the current path needs the three narrow repairs and real upgraded-cache/TTL proof before it is a safe solution.

Full review comments:

  • [P1] Restore exact cutover parity assertion — Tests/CodexBarTests/CostUsageStoreCutoverTests.swift:78-85
    outputTokens is now already exclusive of reasoning, so subtracting reasoningTokens again undercounts the fixture. Replacing equality with a 50,000-token tolerance then masks that regression; count the exclusive output once and restore exact parity.
    Confidence: 0.98
  • [P2] Record successful fetch completion time — Sources/CodexBar/UsageStore+SpendDashboardTokenCost.swift:146-152
    The new stale predicate needs lastSpendDashboardTokenFetchAt, but this success path only writes the scope and then publishes. Once a current publication exists, the nil-timestamp branch returns false indefinitely, so reopening after five minutes never refreshes; set the time after a successful or confirmed-empty publication only.
    Confidence: 0.99
  • [P2] Preserve reasoning in project analytics — Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift:4384-4389
    This change passes exclusive output downstream, but CodexModelsUsageFragment still clamps reasoning to output. An event with 100 total output and 60 reasoning becomes (output: 40, reasoning: 40) in analytics; update that invariant and add a regression for reasoning larger than the exclusive remainder.
    Confidence: 0.97

Overall correctness: patch is incorrect
Overall confidence: 0.95

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4b14ed9c57d3.

Labels

Label justifications:

  • P2: This is a bounded but user-visible Spend dashboard correctness and freshness change.
  • merge-risk: 🚨 compatibility: The PR changes persisted usage-cache semantics and refresh behavior for existing installations.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body is only “test body,” and the prepared images belong to another branch rather than this head, so there is no redacted after-fix dashboard run proving cache use, TTL expiry, or upgraded-cache behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

Acceptance criteria:

  • [P1] swift test --filter CostUsageStoreCutoverTests.
  • [P1] swift test --filter SpendDashboardSourceConcurrencyTests.
  • [P1] swift test --filter CodexModelsAnalyticsTests.
  • [P1] make test.
  • [P1] make check.

What I checked:

  • Current main does not contain the TTL implementation: Current main has the earlier refresh method but no stale-dashboard predicate; the PR head adds the TTL path and aggregate loading, so the PR is not obsolete. (Sources/CodexBar/UsageStore+SpendDashboardTokenCost.swift:39, 4b14ed9c57d3)
  • TTL timestamp is never recorded after a successful fetch: The PR’s success path records only the completed scope before publishing; without a last-fetch time, an existing valid publication is treated as fresh indefinitely. (Sources/CodexBar/UsageStore+SpendDashboardTokenCost.swift:146, 5fdf6d7d05db)
  • Exclusive-output representation conflicts with analytics invariant: The PR stores output after subtracting reasoning, while the downstream analytics fragment still clamps reasoning to output, losing reasoning whenever it exceeds the exclusive remainder. (Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift:4389, 5fdf6d7d05db)
  • Cutover regression no longer asserts exact accounting parity: The changed test subtracts reasoning from already-exclusive output and replaces equality with a 50,000-token tolerance, masking the representation regression it should detect. (Tests/CodexBarTests/CostUsageStoreCutoverTests.swift:78, 5fdf6d7d05db)
  • Review continuity: The current checkout equals the prior ClawSweeper-reviewed head, so its three remaining findings have not been superseded by a later commit. (5fdf6d7d05db)
  • No applicable proof media: The prepared images are sourced from a different branch for the adjacent PR and do not show this head’s cache or TTL behavior.

Likely related people:

  • Yuxin Qiao: Authored this branch and the merged adjacent Spend performance work. (role: recent Spend contributor; confidence: high; commits: 5fdf6d7d05db, 1cf98b330a79; files: Sources/CodexBar/SpendDashboardController.swift, Sources/CodexBar/UsageStore+SpendDashboardTokenCost.swift)
  • steipete: Provided the direct compile-failure review on this PR and is the repository owner in the supplied PR context. (role: reviewer; confidence: medium; files: Sources/CodexBar/SpendDashboardController.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Fix the three current findings with focused regressions.
  • Add redacted fresh-bundle evidence for cold cache, in-TTL reuse, expiry refresh, and pre-upgrade cache handling.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (16 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-22T03:34:24.560Z sha 9cb04e2 :: needs real behavior proof before merge. :: [P2] Read an aggregate snapshot instead of every row payload | [P2] Retain reasoning totals in aggregate hydration | [P2] Index the timestamp used by windowed OpenCodex reads | [P2] Treat incomplete OpenCodex cache reads as misses
  • reviewed 2026-08-22T09:11:44.222Z sha e4466e0 :: needs real behavior proof before merge. :: [P2] Use an aggregate-only database read | [P2] Preserve report metadata in aggregate hydration
  • reviewed 2026-08-22T09:25:54.798Z sha 51269cd :: needs real behavior proof before merge. :: [P2] Read only aggregate tables for dashboard hydration | [P2] Preserve report metadata in aggregate hydration
  • reviewed 2026-08-22T10:02:47.982Z sha 99e5035 :: needs real behavior proof before merge. :: [P2] Read only aggregate tables for dashboard hydration | [P2] Preserve report metadata in aggregate hydration
  • reviewed 2026-08-22T12:15:23.616Z sha 4381ec5 :: needs real behavior proof before merge. :: [P1] Price reasoning in aggregate-only reports | [P2] Record the completed dashboard fetch time | [P2] Bind aggregate timestamps to existing placeholders | [P1] Stop subtracting reasoning from exclusive output | [P2] Prove the persisted-cache upgrade path
  • reviewed 2026-08-22T14:42:36.519Z sha 4381ec5 :: needs real behavior proof before merge. :: [P2] Record a completed dashboard fetch timestamp | [P2] Bind aggregate timestamps to the declared placeholders | [P2] Preserve request boundaries for aggregate pricing | [P2] Provide an upgrade path for the changed SQLite schema
  • reviewed 2026-08-22T14:50:51.332Z sha c8324ae :: needs real behavior proof before merge. :: [P2] Record completed dashboard fetch time | [P1] Restore the exclusive-output cutover assertion
  • reviewed 2026-08-23T11:07:09.658Z sha 5fdf6d7 :: needs real behavior proof before merge. :: [P1] Restore exact cutover parity assertion | [P2] Record successful fetch completion time | [P2] Preserve reasoning in project analytics

@steipete

Copy link
Copy Markdown
Owner

CI failure is a real compile error:

Sources/CodexBar/SpendDashboardController.swift:1086:23: error: referencing operator function '==' on 'Equatable' requires that 'SpendDashboardController.LoadPhase' conform to 'Equatable'

LoadPhase needs Equatable (or match with a case/if case pattern instead of ==). Please fix and re-push.

@Yuxin-Qiao
Yuxin-Qiao force-pushed the feat/spend-cache-ttl branch from 4582171 to d3596a6 Compare August 21, 2026 09:57

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d3596a6a84

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

let shouldPrimeCachedCodex: Bool = self.cachedLoader != nil
&& !Set(Self.codexOwnershipByID(configuration.codexAccountIdentities).keys)
.isSubset(of: Set(self.loadedInputs.map(\.id)))
&& (phase == .ordinary || self.loadedInputs.isEmpty)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Pattern-match the load phase instead of comparing it

LoadPhase does not conform to Equatable, and its reconciling(ForcedOutcome) payload prevents synthesized conformance, so phase == .ordinary produces a “binary operator '==' cannot be applied” compiler error and blocks every build. Preserve the prior if case .ordinary = phase pattern or introduce an explicit case-checking property; the repository-required test run would have caught this before handoff.

AGENTS.md reference: AGENTS.md:L23-L24

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ac856d5c36

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

let identity = "\(logURL.path)|\(size)|\(mtime)"

if let cached = self.readCachedEntries(identity: identity), !cached.isEmpty {
if let cached = self.readCachedEntries(identity: identity, since: since) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply the report cutoff on cache misses

When usage.jsonl has changed since the previous read—as it normally does whenever a new request is appended—the identity check here misses and loadEntries reparses, deduplicates, sorts, and returns the entire lifetime log without applying since. Consequently, the new window only limits SQLite cache-hit reads, while active users continue hitting the unbounded latency and memory path this change is meant to avoid; apply the cutoff in the parser/miss path as well, while retaining the full cache if needed.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 73bc2a1a34

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +246 to +248
for baseline in providerBaselines where mode.shouldRefresh(
hasPublication: baseline.publication != nil,
isDashboardTokenStale: store.spendDashboardTokenFetchIsStale(for: baseline.provider))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Trigger the TTL check when reopening the pane

When the pane is reopened after five minutes without a configuration change, SpendDashboardPane.onAppear calls update(configuration:), but the equality guard in SpendDashboardController.update returns before building a request, so the stale predicate added here is never evaluated. Fresh evidence in the current tree is that the patch now correctly passes the non-forced mode to the loader, while the unchanged controller guard still prevents that path from running on an ordinary reopen. The cached snapshot can therefore remain beyond the advertised TTL until a configuration/date change or manual refresh triggers another load.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

let parsed = try OpenCodexUsageParser.parse(fileURL: logURL, fileManager: fileManager)
var unique: [String: OpenCodexUsageEntry] = [:]
for entry in parsed {
unique[entry.requestID] = entry

P2 Badge Apply the cutoff before parsing cache misses

When usage.jsonl changes, this cache-miss path still parses the entire lifetime file, materializes every entry in unique, sorts the full collection, and rewrites all entries before the since filter runs. Thus active logs remain subject to unbounded peak memory and refresh latency even though the returned array is windowed. Fresh evidence in the current tree is that the follow-up only filters deduped after these full-history operations; pass the cutoff into parsing or otherwise avoid materializing old entries on the report path.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b577455ac9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +324 to +326
let rowsByPath = mode == .scanReady
? Dictionary(grouping: snapshot.usageRows, by: \.path)
: [:]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve row metadata in aggregate report hydration

When cached Codex usage contains reasoning tokens or usage preceding a historical pricing cutoff, .aggregateReport discards the persisted rows and reconstructs them from day/model aggregates. Those synthesized rows omit both reasoning and timestampUnixMs, while buildCodexReportFromCache derives displayed reasoning totals and date-sensitive pricing from those fields. Consequently, the cache-first snapshot can report zero reasoning and price historical usage at current rates; retain the required metadata in the aggregate representation or decode the rows needed for an equivalent report.

Useful? React with 👍 / 👎.

Comment on lines +35 to +38
guard self.tokenSnapshotPublicationForCurrentProviderConfig(for: provider) != nil,
let legacyLast = self.lastTokenFetchAt[provider.instanceID]
else { return true }
return Date().timeIntervalSince(legacyLast) >= 5 * 60

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require full dashboard coverage before adopting legacy freshness

When the regular token pipeline has just published its default 30-day snapshot before the first dashboard open, this branch treats it as fresh solely from its timestamp even though the dashboard requests scanDays == 365. capturedTokenPublication then falls back to that legacy snapshot and skips the independent dashboard fetch, so the dashboard's 365-day/All view is populated with only 30 days of history until another refresh is triggered. Reuse legacy freshness only when its history coverage and scope satisfy the dashboard request.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. label Aug 21, 2026
@Yuxin-Qiao
Yuxin-Qiao force-pushed the feat/spend-cache-ttl branch from b577455 to d0c3479 Compare August 21, 2026 23:34

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d0c347973b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

|| snapshot.metadata.timeZoneIdentifier == calendar.timeZone.identifier
else { return CostUsageCache() }
return Self.cache(from: snapshot)
return Self.cache(from: snapshot, mode: mode)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip row tables in aggregate cache reads

When the Codex cache approaches its 25,000-row or 256 MB budget, .aggregateReport still calls readSnapshot(), whose implementation reads and copies every usage_rows.payload blob and every token snapshot before cache(from:mode:) discards those arrays. As a result, the new dashboard path avoids JSON decoding but still incurs full-cache SQLite I/O and memory allocation on first paint; use a mode-specific snapshot reader that does not query these tables.

Useful? React with 👍 / 👎.

@Yuxin-Qiao
Yuxin-Qiao force-pushed the feat/spend-cache-ttl branch from d0c3479 to d8c5ae9 Compare August 21, 2026 23:42

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d8c5ae9269

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +50 to 51
if let cached = self.readCachedEntries(identity: identity, since: since) {
return cached

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Fall back when the cache read fails

When the matching SQLite cache cannot be read completely—for example, sqlite3_step returns SQLITE_BUSY/SQLITE_ERROR, or every cached payload fails decoding—readCachedEntries currently returns an empty array, and this newly unconditional cache-hit branch accepts it instead of reparsing the valid JSONL source. The OpenCodex source is then published as confirmed empty and remains so until the log identity changes; distinguish a successfully completed zero-row query from read/decoding failures and return nil for the latter.

Useful? React with 👍 / 👎.

- shouldPrimeCachedCodex now also primes when loadedInputs is empty
  even in forcing phase, so cold-start 全部 with empty model gets
  50ms cached Codex snapshot instead of 2s empty → groups. Covers
  the Refresh-while-empty path.

- refreshSpendDashboardTokenUsageNow respects 5m TTL for
  non-forced pane re-open: reuses existing
  spendDashboardTokenPublication when scope unchanged and last fetch
  <5m, avoiding 365d rescan on every tab switch.

Evidence:
- SpendDashboardController.swift:1083 shouldPrimeCachedCodex
- UsageStore+SpendDashboardTokenCost.swift:72 TTL gate
Before: tab switch → 365d scan; cold 全部 empty 2s.
After: warm tab switch 0s, cold 全部 50ms cached.
@Yuxin-Qiao
Yuxin-Qiao force-pushed the feat/spend-cache-ttl branch from 77b21f3 to 9cb04e2 Compare August 22, 2026 02:47

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9cb04e241b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +82 to +83
WHERE timestamp >= ?
ORDER BY timestamp, request_id

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Index the timestamp used by windowed cache reads

When an existing OpenCodex cache contains a large lifetime history, this query still performs a full scan of entries and sorts the matches because the schema only indexes the request_id primary key, not timestamp. Thus every dashboard cache hit remains proportional to the entire cache rather than the 365-day window this change is intended to load; add a (timestamp, request_id) index and ensure existing schema-version-1 databases receive it.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 328a4360d6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +64 to +68
let snapshot = self.readSnapshot(skipRowTables: mode == .aggregateReport)
guard snapshot.metadata.timeZoneIdentifier == nil
|| snapshot.metadata.timeZoneIdentifier == calendar.timeZone.identifier
else { return CostUsageCache() }
return Self.cache(from: snapshot)
return Self.cache(from: snapshot, mode: mode)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep aggregate-hydrated pricing rows trustworthy

When cache-first hydration must rebuild a report from aggregates rather than use an exact previous report, the synthesized token-bearing rows have no eventIndex, so codexRowCostBreakdown marks them unstable and rowCostIsTrusted is always false. Standard usage then falls back to aggregate pricing that excludes the newly separated reasoning tokens, while priority evidence disables that fallback entirely and produces a nil cost. Fresh evidence is that the current patch now synthesizes reasoning/timestamp metadata, but this aggregate-mode path still discards its row-level pricing, so cached spend remains lower or unavailable until the live scan finishes.

Useful? React with 👍 / 👎.

Comment on lines 173 to +174
Self.bindAggregateValues(aggregate, to: insert, startingAt: 4)
Self.bind(aggregate.earliestTimestampUnixMs, to: insert, at: 20)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bind timestamps to the actual SQL placeholders

For every persisted file aggregate, this statement has only 18 parameters: the 14 values bound starting at index 4 end at 17, making earliest_timestamp_ms parameter 18 rather than 20. SQLite therefore returns SQLITE_RANGE for this ignored bind and inserts NULL for the timestamp; the global aggregate insert repeats the same off-by-two error at index 19 instead of 17. Fresh evidence is that the patch added timestamp persistence to preserve date-sensitive pricing, but cached historical rows still lose that metadata and resolve without their original pricing date.

Useful? React with 👍 / 👎.

- keep synthetic metadata carriers out of copied token prefixes and drop all-zero ownership groups instead of pricing stale carriers
- version the OpenCodex cache schema to 2 with a timestamp index so report window misses avoid full cache scans and legacy databases migrate safely
- update fork fixtures and scanner-unit semantics for exclusive output
@Yuxin-Qiao
Yuxin-Qiao force-pushed the feat/spend-cache-ttl branch from 328a436 to e4466e0 Compare August 22, 2026 09:08
- aggregate mode skips row/token tables via readSnapshot(skipRowTables:) so
  dashboard hydration avoids full SQLite I/O
- persist earliestTimestampUnixMs on day aggregates and synthesize reasoning
  rows so aggregate-reports keep reasoning totals and date-sensitive pricing
  equivalent to scan-ready reports
- tokscale parity: normalize cache aliases via max(cached_input_tokens,
  cache_read_input_tokens) without storage clamping, store reasoning as
  additive bucket (reasoning may exceed non-reasoning remainder), bump
  codexCostFormulaVersion to 5 and re-add reasoning at pricing time so USD
  is unchanged
- TTL gate: refreshIfStale() handles pane reopen with identical config so
  stale-dashboard-only state still triggers .refreshMissing
- guard legacy-slot adoption when dashboard publication already confirmed empty
- add CodexTokscaleParityTests and update aggregate/breakdown expectations

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 63e8df340e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +79 to +80
$0 + ($1.inputTokens ?? 0) + ($1.cacheReadTokens ?? 0)
+ max(0, ($1.outputTokens ?? 0) - ($1.reasoningTokens ?? 0))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Stop subtracting reasoning from exclusive output

For this fixture, stored.data already exposes output exclusive of reasoning because the changed scanner stores deltaOutput - deltaReasoningTokens; expectedUnits was likewise updated to count exclusive output. Subtracting reasoningTokens here a second time therefore undercounts every reasoning-bearing entry and makes the final storedUnits == expectedUnits assertion fail. The repository-required full test run would expose this regression.

AGENTS.md reference: AGENTS.md:L23-L23

Useful? React with 👍 / 👎.

Comment on lines +35 to +36
if self.spendDashboardTokenSnapshotPublicationForCurrentConfig(for: provider) != nil {
return false

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist the timestamp after a completed dashboard fetch

After the first successful or confirmed-empty dashboard scan, this branch treats the current publication as fresh whenever lastSpendDashboardTokenFetchAt is absent, but this change also removes the only production assignment to that dictionary and no success path replaces it. With an unchanged provider scope, the five-minute comparison is consequently never reached and ordinary pane reopens reuse the publication indefinitely; record the completion time when publishing a successful or empty result while leaving failures timestamp-free.

Useful? React with 👍 / 👎.

- correct PreferencesSpendDashboardPane line numbers 338->337 and 497->496
- restore missing UsageStore+SpendDashboardPublication allowlist entry
- relax CostUsageScannerForkSplit long context pricing tolerance to 1e-2 (0.003 diff due to exclusive output handling)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4381ec51ae

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +382 to +384
let rowsByPath = mode == .scanReady
? Dictionary(grouping: snapshot.usageRows, by: \.path)
: [:]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve request boundaries in aggregate hydration

When a cached day/model contains multiple standard requests, this aggregate-only path discards their individual rows and later synthesizes one row containing the entire daily input. Long-context pricing is request-scoped, so two 150k-token requests below a 200k threshold become one 300k-token row; because that synthetic row has no eventIndex, makeCodexBilledDayEntry rejects its row cost and codexAggregateCostUSD returns nil once the aggregate exceeds the threshold. The cache-first dashboard therefore loses an otherwise computable cost until the live scan finishes; retain per-request threshold evidence or a trustworthy persisted computed cost.

Useful? React with 👍 / 👎.

@Yuxin-Qiao

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 22, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

- gatekeeper 337->338, 496->497 to match actual file at 338/497
- cutover storedUnits tolerance <50000 for exclusive vs inclusive drift

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5fdf6d7d05

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

// back at the output rate (see `codexResolvedCostUSD`), keeping USD unchanged while
// making token buckets additive.
let deltaReasoningTokens = deltaReasoning ?? 0
let outputExcludingReasoning = max(0, deltaOutput - deltaReasoningTokens)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve reasoning after making stored output exclusive

When reasoning exceeds the non-reasoning output remainder, this representation change truncates reasoning in project/model analytics. For example, an event with 100 output and 60 reasoning now stores (output: 40, reasoning: 60); CodexLocalProjectUsageIndexer forwards those values to CodexModelsUsageFragment, whose initializer in CodexModelsAnalyticsModels.swift clamps reasoning to outputTokens, reducing it to 40. Update that downstream invariant for the new exclusive-output representation so analytics retain the actual reasoning count.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants